Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert achievement ii #546

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Revert achievement ii #546

wants to merge 2 commits into from

Conversation

dhenkel92
Copy link
Member

No description provided.

@Jonasdoubleyou Jonasdoubleyou temporarily deployed to user-app-revert-achieve-j9p2vu May 30, 2024 11:13 Inactive
@dhenkel92 dhenkel92 force-pushed the revert-achievement-ii branch 2 times, most recently from 59b0fd0 to 1b2f350 Compare September 8, 2024 14:13
@dhenkel92 dhenkel92 temporarily deployed to user-app-revert-achieve-m8qo7z September 8, 2024 14:27 Inactive
@dhenkel92 dhenkel92 temporarily deployed to user-app-revert-achieve-m8qo7z September 11, 2024 18:37 Inactive
@@ -135,6 +160,7 @@ const ImportantInformation: React.FC<Props> = ({ variant }) => {
const pupil = data?.me?.pupil;
const student = data?.me?.student;
const email = data?.me?.email;
const nextStepAchievements: Achievement[] = !GAMIFICATION_ACTIVE ? [] : data?.me.nextStepAchievements ?? [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove the feature toggle

progressDescription
actionName
actionRedirectLink
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe as an additional safeguard (and I think it generally makes sense) we could maybe slightly defer the ImportantInformation query a bit (useLazyQuery + useEffect with timeout that triggers it) so that other parts of the page load first and then we have plenty of time to fetch these? For sure we would then make sure that adding the important notifications does not cause a layout shift ...

@@ -289,6 +320,8 @@ const ImportantInformation: React.FC<Props> = ({ variant }) => {
});

// -------- Certificate of Conduct -----------
// TODO - remove if achievements are included [ONBOARDING]?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, need to keep it - Support can request this even if the user is not currently registering - Or would this show up as a ghost?

@@ -190,6 +218,7 @@ const ImportantInformation: React.FC<Props> = ({ variant }) => {
});

// -------- Screening -----------
// TODO - remove if achievements are included
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if already screened users request another screening - e.g. students screened as tutors want to be screened as instructors?

@Jonasdoubleyou
Copy link
Member

Screenshot 2024-09-11 at 21 30 09

The achievement cards lack a subtitle ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants